Skip to content

Test: add URL to re2 rubygems.org page to assert#58

Merged
mattyoho merged 1 commit intogithub:masterfrom
olleolleolle:fix/add-link-to-description
Jan 25, 2019
Merged

Test: add URL to re2 rubygems.org page to assert#58
mattyoho merged 1 commit intogithub:masterfrom
olleolleolle:fix/add-link-to-description

Conversation

@olleolleolle
Copy link
Copy Markdown
Contributor

@olleolleolle olleolleolle commented Nov 19, 2016

This PR adds the rubygems.org URL to the re2 gem to the assert failure message that tells you to install it.

  - adds armchair "developer experience"
@mattyoho mattyoho self-assigned this Jan 25, 2019
Copy link
Copy Markdown
Contributor

@mattyoho mattyoho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the ergonomics patch!

t0 = Time.now
email("pathological")
assert (Time.now - t0) < 1, "Took too long, upgrade to re2 gem."
assert (Time.now - t0) < 1, "Took too long, upgrade to re2 gem. See https://rubygems.org/gems/re2"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slightly tweaking the error message language:

Suggested change
assert (Time.now - t0) < 1, "Took too long, upgrade to re2 gem. See https://rubygems.org/gems/re2"
assert (Time.now - t0) < 1, "Parsing took too long, please install the re2 gem locally. See https://rubygems.org/gems/re2"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, maintainers aren't able to edit the upstream. 🙂Accepting as-is. 👍

@mattyoho mattyoho merged commit 0ba0600 into github:master Jan 25, 2019
@olleolleolle olleolleolle deleted the fix/add-link-to-description branch January 25, 2019 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants